Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update testcafe version to 3.6.2 #28075

Draft
wants to merge 42 commits into
base: 24_2
Choose a base branch
from

Conversation

ivanblinov2k17
Copy link
Contributor

No description provided.

@@ -9,6 +9,10 @@ import { appendElementTo } from '../../helpers/domUtils';
fixture.disablePageReloads`ChatAvatar`
.page(url(__dirname, '../container.html'));

test('Native automation check', async t => {
await t.expect(t.browser.nativeAutomation).ok();
//the test continues only if you use native automation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this change.
Why it should be enabled if u disable it in a general config?
Why this test should be exactly for chat Avatar?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be deleted, artifact of testing phase

@@ -159,9 +159,9 @@ function setShadowDom(args) {
function expandBrowserAlias(browser) {
switch(browser) {
case 'chrome:devextreme-shr2':
return 'chrome:headless --disable-gpu --window-size=1200,800';
return 'chrome:headless --disable-gpu --window-size=1200,1000';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm., why?
Should be reviewed by @EugeniyKiyashko

break;
default:
sizes = [1200, 800];
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's very strange for me.
At least it can be refactored to reduce copypaste.
But I'm not quire sure it's okay to specify component-specific sized on a runner level

@@ -2,9 +2,9 @@
type BrowserSizeType = [width: number, height: number];
const DEFAULT_BROWSER_SIZE: BrowserSizeType = [1200, 800];

const restoreBrowserSize = async (t: TestController):
const restoreBrowserSize = async (t: TestController, sizeRestoreTo?: BrowserSizeType):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's ok you're specifying non-default size for form test on runner level and then extend restoreBrowserSize to restore it to a non-default size manually

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should understand why root input hover indication is missing now

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be a flaky scenario

@Alyar666 Alyar666 self-requested a review October 4, 2024 10:54
@@ -4,7 +4,8 @@
],
"screenshots": {
"path": "./",
"pathPattern": "${TEST}.png"
"pathPattern": "${TEST}.png",
"textDiffTreshold": 0.1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For what scenarios is this setting necessary? Does it affect all etalons and does it not work without it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be a flaky scenario

@@ -126,7 +126,7 @@
"stylelint-config-recommended-vue": "1.5.0",
"stylelint-config-standard": "35.0.0",
"systemjs-builder": "0.16.15",
"testcafe": "2.6.2",
"testcafe": "3.6.2",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we separate the version update for the demos and the DevExtreme repository? In this case, the changes will be more obvious.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants